Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow other runtimes to use the Hermes Debugger plugin for React Native #4047

Closed
wants to merge 1 commit into from

Conversation

Kwasow
Copy link

@Kwasow Kwasow commented Sep 1, 2022

Motivation

Hi, I'm a member of the react-native-reanimated team at Software Mansion, where we develop a module for React Native. Due to the architecture of this module, we need to create our own runtime. It has long been the case that code being executed in our runtime environment, could not have been debugged using most debugging tools. This is due to change soon, as the work done in this PR enables full support for debugging using Chrome DevTools, when using Hermes as the runtime.

Unfortunately, this runtime is not visible to the Hermes Debugger plugin as it's name (Reanimated runtime) is different from the hard-coded name of React Native's reload friendly runtime (React Native Experimental (Improved Chrome Reloads)) and therefore cannot be debugged using this method.

Solutions

  1. We (at Software Mansion) could create our own fork of the Hermes Debugger plugin, where we would replace the hard-coded runtime name with the name of our own runtime.

  2. The code in the Hermes Debugger plugin could be updated to show all targets, which names include (Improved Chrome Reloads). Then any module, which creates its own runtime, could support Chrome reloads and by indicating that in the name would be shown by the Hermes Debugger plugin.

I believe the second solution to be the preferred one, as it enables support for more than just our single use-case, while also adding minimal maintenance work both for Flipper's developers and the community creating modules.

Other suggestions

If this change is implemented, it would be a good idea to have a way to close the debugger and go back to the list of available runtimes (ex. a back button), but I think this is out of the scope of this PR.

Changes

Before:

const targets = result.filter(
  (target: any) =>
    target.title ===
    'React Native Experimental (Improved Chrome Reloads)',
);

After:

const targets = result.filter(
  (target: any) =>
    target.title.includes('(Improved Chrome Reloads)'),
);

Test Plan

This change does not require any new testing.

Related PRs

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 1, 2022
@Kwasow Kwasow changed the title Proposal: Allow other runtimes to use the Hermes Debugger plugin for React Native Allow other runtimes to use the Hermes Debugger plugin for React Native Sep 1, 2022
@Kwasow
Copy link
Author

Kwasow commented Sep 14, 2022

After discussing this internally, we don't want our issue to be solved this way, and we believe that a separate plugin will create a more user-friendly experience.

@Kwasow Kwasow closed this Sep 14, 2022
piaskowyk pushed a commit to software-mansion/react-native-reanimated that referenced this pull request Sep 22, 2022
## Description

This PR is related to #1960 (and its replacement #2047). It adds the
ability to use Chrome DevTools to add breakpoints and debug worklets (or
the UI context in general) both on Android and iOS.

## Major changes

Runtime creation has been moved to `ReanimatedRuntime` for both Android
and iOS (in the `Common/cpp/ReanimatedRuntime` directory).

Before (Android) [file: `NativeProxy.cpp`]:
```cpp
#if JS_RUNTIME_HERMES
  auto config =
      ::hermes::vm::RuntimeConfig::Builder().withEnableSampleProfiling(false);
  std::shared_ptr<jsi::Runtime> animatedRuntime =
      facebook::hermes::makeHermesRuntime(config.build());
#elif JS_RUNTIME_V8
      …
#else
      …
#endif
```

After (shared) [file: `NativeProxy.cpp`]:
```cpp
ReanimatedRuntime::make(jsQueue);
```

The custom build config has been removed, as sample profiling is set to
false by default.

The created `HermesRuntimeManager` object is the stored inside
`ReanimatedNativeModule` as it is important that it’s lifetime is synced
with the lifetime of the module.

## Testing

- [x] Builds on Android
- [x] Builds on Android in release mode
- [x] App reloads work on Android
- [x] Builds on iOS
- [x] Builds on iOS in release mode
- [x] App reloads work on iOS
- [x] JSC still works
- [x] JSC debugging in Safari with iOS still works
- [x] Paper still works

Versions of React-Native tested: 0.70

## Things to look into

- ~~Breakpoint labels do not appear on iOS~~
_I tested this on a new app with Flipper on the main JS thread, and it
was also the case, so it seem to not be an issue on our side_
- ~~After removing a breakpoint on iOS it can't be set in the same
location again~~
_This seems to be an issue with Chrome DevTools as connecting to
Reanimated's runtime through Flipper fully works_
- ~~Edge-case: the app will crash if a reload is performed while the
debugger is open~~
_Will be fixed in a PR to metro and
[58e9e7b](https://github.com/software-mansion/react-native-reanimated/pull/3526/commits/58e9e7bcd7d3b6b590a5f6fca0db93a951eaa39e)_
- The latest release of Chrome (105) broke source maps, so only Flipper
works now. I reported the issue
[here](https://bugs.chromium.org/p/chromium/issues/detail?id=1360298#makechanges),
and I'm waiting for a response from the Chromium team

## TODO

- [x] Open PR to update debugging docs in documentation (will be
included in #3446)
- [x] Open PR in [facebook/flipper](https://github.com/facebook/flipper)
to enable Flipper support on custom runtimes (facebook/flipper#4047)
- [x] Open PR in [facebook/metro](https://github.com/facebook/metro) to
support debugger reloads on custom runtimes (facebook/metro#864)
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
…are-mansion#3526)

## Description

This PR is related to software-mansion#1960 (and its replacement software-mansion#2047). It adds the
ability to use Chrome DevTools to add breakpoints and debug worklets (or
the UI context in general) both on Android and iOS.

## Major changes

Runtime creation has been moved to `ReanimatedRuntime` for both Android
and iOS (in the `Common/cpp/ReanimatedRuntime` directory).

Before (Android) [file: `NativeProxy.cpp`]:
```cpp
#if JS_RUNTIME_HERMES
  auto config =
      ::hermes::vm::RuntimeConfig::Builder().withEnableSampleProfiling(false);
  std::shared_ptr<jsi::Runtime> animatedRuntime =
      facebook::hermes::makeHermesRuntime(config.build());
#elif JS_RUNTIME_V8
      …
#else
      …
#endif
```

After (shared) [file: `NativeProxy.cpp`]:
```cpp
ReanimatedRuntime::make(jsQueue);
```

The custom build config has been removed, as sample profiling is set to
false by default.

The created `HermesRuntimeManager` object is the stored inside
`ReanimatedNativeModule` as it is important that it’s lifetime is synced
with the lifetime of the module.

## Testing

- [x] Builds on Android
- [x] Builds on Android in release mode
- [x] App reloads work on Android
- [x] Builds on iOS
- [x] Builds on iOS in release mode
- [x] App reloads work on iOS
- [x] JSC still works
- [x] JSC debugging in Safari with iOS still works
- [x] Paper still works

Versions of React-Native tested: 0.70

## Things to look into

- ~~Breakpoint labels do not appear on iOS~~
_I tested this on a new app with Flipper on the main JS thread, and it
was also the case, so it seem to not be an issue on our side_
- ~~After removing a breakpoint on iOS it can't be set in the same
location again~~
_This seems to be an issue with Chrome DevTools as connecting to
Reanimated's runtime through Flipper fully works_
- ~~Edge-case: the app will crash if a reload is performed while the
debugger is open~~
_Will be fixed in a PR to metro and
[58e9e7b](https://github.com/software-mansion/react-native-reanimated/pull/3526/commits/58e9e7bcd7d3b6b590a5f6fca0db93a951eaa39e)_
- The latest release of Chrome (105) broke source maps, so only Flipper
works now. I reported the issue
[here](https://bugs.chromium.org/p/chromium/issues/detail?id=1360298#makechanges),
and I'm waiting for a response from the Chromium team

## TODO

- [x] Open PR to update debugging docs in documentation (will be
included in software-mansion#3446)
- [x] Open PR in [facebook/flipper](https://github.com/facebook/flipper)
to enable Flipper support on custom runtimes (facebook/flipper#4047)
- [x] Open PR in [facebook/metro](https://github.com/facebook/metro) to
support debugger reloads on custom runtimes (facebook/metro#864)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants